Skip to content

fix(client): preserve agent_uri trailing slash; widen MCP URL fallbacks#582

Merged
bokelley merged 1 commit intomainfrom
claude/issue-581-trailing-slash
May 6, 2026
Merged

fix(client): preserve agent_uri trailing slash; widen MCP URL fallbacks#582
bokelley merged 1 commit intomainfrom
claude/issue-581-trailing-slash

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 5, 2026

Closes #581

Summary

validate_agent_uri in AgentConfig was calling v.rstrip("/") before storing the URI, silently dropping path trailing slashes. FastMCP servers (and Starlette Mount-based servers generally) mount their streamable-HTTP endpoint at /mcp/ and return 404 to /mcp. The client couldn't connect to servers built with the project's own server-side helpers.

Three changes:

  1. src/adcp/types/core.py — remove rstrip("/") from validate_agent_uri; preserve the caller-supplied URI exactly after the scheme check.

  2. src/adcp/protocols/mcp.py — update urls_to_try construction so both /mcp and /mcp/ forms are tried regardless of which the user supplied. Previously only a no-slash /mcp fallback was appended.

  3. src/adcp/signing/capability_cache.py — normalize trailing slash inside build_capability_cache_key so http://host/mcp and http://host/mcp/ produce the same cache entry. Without this, the same logical agent would produce different cache keys depending on whether the caller supplied a trailing slash.

Upgrade note

AgentConfig.agent_uri now stores the value exactly as supplied. If your code reads back config.agent_uri and compares it to a hard-coded no-slash string (e.g. == "https://host/mcp"), update that comparison or strip the slash at the comparison site.

What was tested

  • pytest tests/test_client.py::test_agent_uri_preserves_user_supplied_form — 4 parametrized validator round-trip cases
  • pytest tests/test_protocols.py::TestMCPUrlFallback — 4 parametrized URL-fallback cases covering /mcp/, /mcp, bare host, host-with-slash
  • Updated 2 fixture assertions in test_helpers.py
  • Full non-integration suite: 4090 passed, 30 skipped (1 pre-existing network flake in test_ip_pinned_transport.py::test_real_tls_handshake_still_validates_hostname excluded — hits example.com, unrelated to this change)
  • ruff check src/: all checks passed

Pre-PR review:

  • code-reviewer: approved — ternary logic correct for all four URI forms; nit: ternary reads slightly backwards (f"{base}/" if not uri.endswith("/") else base), consider base if uri.endswith("/") else f"{base}/" for readability
  • dx-expert: approved — error log now shows user-supplied URI (genuine DX improvement); nit: build_capability_cache_key docstring "matches JS SDK exactly" may be stale after slash normalization; nit: no test asserting key("https://x/mcp/") == key("https://x/mcp")

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01DDfduQk3gWobDVbG8Ct6jk


Generated by Claude Code

@bokelley bokelley marked this pull request as ready for review May 6, 2026 18:14
Closes #581

`validate_agent_uri` in `AgentConfig` was calling `v.rstrip("/")`,
silently dropping path trailing slashes. FastMCP servers (and Starlette
`Mount`-based servers generally) mount their streamable-HTTP endpoint at
`/mcp/` and return 404 to `/mcp`. The client couldn't connect to servers
built with the project's own server-side helpers.

Three changes:

1. `src/adcp/types/core.py` — remove `rstrip("/")` from
   `validate_agent_uri`; preserve the caller-supplied URI exactly after
   the scheme check.

2. `src/adcp/protocols/mcp.py` — update `urls_to_try` so both `/mcp`
   and `/mcp/` forms are tried regardless of which the user supplied.

3. `src/adcp/signing/capability_cache.py` — normalize trailing slash
   inside `build_capability_cache_key` so `http://host/mcp` and
   `http://host/mcp/` produce the same cache entry. Without this, the
   same logical agent would split-brain across two cache entries.

Tests: 4 parametrized AgentConfig validator round-trip cases
(`test_agent_uri_preserves_user_supplied_form`), 4 parametrized URL
fallback cases (`TestMCPUrlFallback`), 3 parametrized cache-key slash
normalization cases (`test_cache_key_normalizes_trailing_slash`), and
2 fixture-assertion updates in `test_helpers.py`.

Upgrade note: `AgentConfig.agent_uri` now stores the value exactly as
supplied. If your code reads back `config.agent_uri` and compares to a
hard-coded no-slash string, update that comparison or strip at the
comparison site.
@bokelley bokelley force-pushed the claude/issue-581-trailing-slash branch from 8d964f6 to c0fd21b Compare May 6, 2026 18:20
@bokelley bokelley merged commit 1db3ce3 into main May 6, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(client): agent_uri Pydantic validator strips trailing slash, breaks connections to FastMCP servers mounted at /mcp/

1 participant